[SPARK-32034][SQL] Port HIVE-14817: Shutdown the SessionManager timeoutChecker thread properly upon shutdown#28870
[SPARK-32034][SQL] Port HIVE-14817: Shutdown the SessionManager timeoutChecker thread properly upon shutdown#28870yaooqinn wants to merge 3 commits intoapache:masterfrom yaooqinn:SPARK-32034
Conversation
…utChecker thread properly upon shutdown
|
Test build #124277 has finished for PR 28870 at commit
|
wangyum
left a comment
There was a problem hiding this comment.
Do we need to backport to hive-1.2?
|
retest this please |
|
Test build #124292 has finished for PR 28870 at commit
|
|
Test build #124293 has finished for PR 28870 at commit
|
| @Override | ||
| public synchronized void stop() { | ||
| super.stop(); | ||
| shutdown = true; |
There was a problem hiding this comment.
This should be removed because we did it in shutdownTimeoutChecker function.
HIVE-14817 removed this, too.
| @Override | ||
| public synchronized void stop() { | ||
| super.stop(); | ||
| shutdown = true; |
There was a problem hiding this comment.
This should be removed because we did it in shutdownTimeoutChecker function.
HIVE-14817 removed this, too.
|
Gentle ping, @yaooqinn . :) |
|
@dongjoon-hyun thanks for the check! |
|
Test build #124331 has finished for PR 28870 at commit
|
dongjoon-hyun
left a comment
There was a problem hiding this comment.
+1, LGTM. Thank you all.
Merged to master/3.0.
|
BTW, @yaooqinn . Is this relevant to |
…utChecker thread properly upon shutdown ### What changes were proposed in this pull request? This PR port https://issues.apache.org/jira/browse/HIVE-14817 for spark thrift server. ### Why are the changes needed? When stopping the HiveServer2, the non-daemon thread stops the server from terminating ```sql "HiveServer2-Background-Pool: Thread-79" #79 prio=5 os_prio=31 tid=0x00007fde26138800 nid=0x13713 waiting on condition [0x0000700010c32000] java.lang.Thread.State: TIMED_WAITING (sleeping) at java.lang.Thread.sleep(Native Method) at org.apache.hive.service.cli.session.SessionManager$1.sleepInterval(SessionManager.java:178) at org.apache.hive.service.cli.session.SessionManager$1.run(SessionManager.java:156) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) at java.lang.Thread.run(Thread.java:748) ``` Here is an example to reproduce: https://github.com/yaooqinn/kyuubi/blob/master/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/spark/SparkSQLEngineApp.scala Also, it causes issues as HIVE-14817 described which ### Does this PR introduce _any_ user-facing change? NO ### How was this patch tested? Passing Jenkins Closes #28870 from yaooqinn/SPARK-32034. Authored-by: Kent Yao <yaooqinn@hotmail.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 9f8e15b) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
yes. need a followup? |
|
If you don't mind, could you make a backporting PR then? |
|
OK |
|
Thank you! |
…timeoutChecker thread properly upon shutdown ### What changes were proposed in this pull request? This PR backports #28870 which ports https://issues.apache.org/jira/browse/HIVE-14817 for spark thrift server. ### Why are the changes needed? Port HIVE-14817 to fix related issues ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? passing Jenkins Closes #28888 from yaooqinn/SPARK-32034-24. Authored-by: Kent Yao <yaooqinn@hotmail.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
|
LGTM, thanks! |
What changes were proposed in this pull request?
This PR port https://issues.apache.org/jira/browse/HIVE-14817 for spark thrift server.
Why are the changes needed?
When stopping the HiveServer2, the non-daemon thread stops the server from terminating
Here is an example to reproduce:
https://github.com/yaooqinn/kyuubi/blob/master/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/spark/SparkSQLEngineApp.scala
Also, it causes issues as HIVE-14817 described which
Does this PR introduce any user-facing change?
NO
How was this patch tested?
Passing Jenkins